quote arguments in run_cli_command before shell execution#47425
Conversation
alhudz
requested review from
JustinFirsching,
NonStatic2014,
achauhan-scc,
arunsu,
jayesh-tanna,
kingernupur,
nick863,
novaturient95,
raduk,
rtanase,
sharma-riti and
vivram
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens local-endpoint CLI execution by correctly quoting command arguments before running them via shell=True, preventing shell interpretation of metacharacters/spaces.
Changes:
- Update
run_cli_commandto build a safely-quoted command string usingshlex.join(POSIX) /subprocess.list2cmdline(Windows). - Add unit tests validating quoting behavior and ensuring a plain URI remains unchanged.
- Document the fix in the package changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/ml/azure-ai-ml/azure/ai/ml/_local_endpoints/utilities/commandline_utility.py | Quote args when collapsing to a single shell command string. |
| sdk/ml/azure-ai-ml/tests/local_endpoint/unittests/test_commandline_utility.py | New unit tests asserting safe quoting and expected command string formatting. |
| sdk/ml/azure-ai-ml/CHANGELOG.md | Changelog entry describing the quoting/security fix. |
| # "shell=True" (used below) is needed so the platform "az"/"code" wrappers resolve, and it doesn't work with | ||
| # the vector argv form on macOS, so the arguments are collapsed into a single command string. Quote each | ||
| # argument while joining so a value containing spaces or shell metacharacters is passed through literally | ||
| # instead of being re-interpreted by the shell. | ||
| if os.name == "nt": | ||
| command_to_execute = subprocess.list2cmdline(cmd_arguments) | ||
| else: | ||
| command_to_execute = shlex.join(cmd_arguments) |
|
|
||
| import json | ||
| import os | ||
| import shlex |
|
Thank you for your contribution @alhudz! We will review the pull request and get back to you soon. |
github-actions
Bot
added
Community Contribution
|
Looks like |
|
any update? |
Description
run_cli_commandjoins its argument vector with spaces and runs the result undersubprocesswithshell=True, so any argument carrying spaces or shell metacharacters is re-interpreted by the shell rather than passed through literally. Callers already hand it a proper argv list; the unsafe collapse happens inside the helper.Repro:
run_cli_command(["echo", "x$(touch pwned)"])runs the$(...)substitution. The VS Code debug path reaches this withAML_APP_ROOT(derived from a deploymentscoring_script/codepath) embedded in the--folder-uriargument, and ordinary paths containing a space break the same way.Cause:
" ".join(cmd_arguments)turns the argv list into a shell string with no quoting.Fix: quote each argument while joining inside the helper,
shlex.joinon POSIX andsubprocess.list2cmdlineon Windows. A plainvscode-remote://URI argument is left unchanged.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines